Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate ginga_plugins. #413

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mkelley
Copy link
Member

@mkelley mkelley commented Sep 11, 2024

Deprecate the CometaryEnhancements class and the ginga_plugins submodule itself. Also, add a deprecation message to the CometaryEnhancements gui, in case users are not watching the console. There is no documentation to edit, as this is a sort of pre-release feature that was never formally documented.

Instead use the sbpy-ginga module: https://github.com/NASA-Planetary-Science/sbpy-ginga

@pep8speaks
Copy link

pep8speaks commented Sep 11, 2024

Hello @mkelley! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-09-11 18:33:46 UTC

Copy link

Thank you for your contribution to sbpy, an Astropy affiliated package! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the sbpy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related?
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label.
  • Is a milestone added?

@mkelley mkelley added this to the v0.6 milestone Sep 11, 2024
Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.

Project coverage is 83.15%. Comparing base (813fe61) to head (7434e75).

Files with missing lines Patch % Lines
sbpy/ginga_plugins/CometaryEnhancements.py 28.57% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #413      +/-   ##
==========================================
- Coverage   83.19%   83.15%   -0.05%     
==========================================
  Files          88       88              
  Lines        8183     8191       +8     
==========================================
+ Hits         6808     6811       +3     
- Misses       1375     1380       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hhsieh00
Copy link
Collaborator

5 lines in CometaryEnhancements.py are marked as not being covered by tests (with the CodeCov report on this page as well as the CodeCov messages on the code diffs page actually consistent this time). Is this a problem or no?

@mkelley
Copy link
Member Author

mkelley commented Sep 12, 2024

Yeah, those lines and that function call can't be tested without running the GUI with ginga. Or, at least, I don't know how to do it. I checked ginga's own continuous integration tests and its coverage and nothing in the reference viewer or its plugins are tested at all. To manually test them, you can pip install this PR and ginga, run ginga, open a file, and start up CometaryEnhancements in the plugins menu. There should be a text box showing the deprecation message.

Copy link
Collaborator

@hhsieh00 hhsieh00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, just getting to this now. I installed ginga, opened it, opened a file, and don't even see CometaryEnhancements in the plugin menu to check for a deprecation message, but if that's the case, it seems that no one should encounter this anyways, so I'm going to call it good.

@mkelley
Copy link
Member Author

mkelley commented Sep 21, 2024

If you open ginga with ginga --stderr is there a line like the following?

2024-09-21 08:34:43,806 | I | PluginManager.py:70 (load_plugin) | Plugin 'CometaryEnhancements' loaded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants